Skip to content

Check every new delete file for rewritten files on a retry - #4374

Open
ghoshp83 wants to merge 1 commit into
apache:mainfrom
ghoshp83:retry-check-keeps-all-delete-files
Open

ghoshp83 wants to merge 1 commit into
apache:mainfrom
ghoshp83:retry-check-keeps-all-delete-files

Conversation

@ghoshp83

Copy link
Copy Markdown
Contributor

Closes #4291.

Rationale for this change

Follow-up to #4292, which handled the rewrite itself. This covers the retry conflict check from the issue.

When a commit retries, _validate_no_new_deletes_for_data_files checks that no new delete file applies to the data files being replaced. It built its delete index with the operation's filter, though, so _filter_manifest_entries skipped any delete file whose column bounds can't match that filter. A partial delete rewrites those data files in full, so a concurrent delete outside the filter went unnoticed. The retry then committed the rewritten file, and the rows that delete removed came back.

Now every delete file added since the starting snapshot is checked against the replaced data files. The filter is no longer used there, so data_filter and case_sensitive are dropped from the private function's signature. _validate_no_new_delete_files, the filter-based check, is unchanged.

Are these changes tested?

Yes. test_partial_delete_validates_concurrent_delete_outside_the_filter in tests/table/test_commit_retry.py runs against each catalog fixture:

  • a stale table handle starts delete("k = 1");
  • meanwhile, a position delete storing the row (2, 'b') is committed. It has bounds k = 2, built with commit_partition_position_delete;
  • the retried delete must raise ValidationException, leaving k = [1, 3].

Without the fix, the test fails with DID NOT RAISE ValidationException, because the retry commits over the concurrent delete.

Are there any user-facing changes?

A partial delete, filtered overwrite or upsert that races with a delete on the same data file now fails validation on retry, instead of silently bringing the deleted rows back.

On a retry, the check for new deletes on the data files being replaced
built its delete index with the operation's filter, so it skipped delete
files whose column bounds can't match the filter. A partial delete
rewrites those files in full, so it could commit over a concurrent delete
outside the filter and bring the deleted rows back.

Check every delete file added since the starting snapshot against the
replaced data files instead. The filter is no longer used there, so drop
it from the signature.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Partial deletes can bring back rows removed by earlier delete files

1 participant